Actionable comments posted: 5

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/bun.js/bindings/ErrorCode.cpp`:
- Around line 590-600: The code does not guard for expected_types.size() == 0
which causes unsigned underflow when the later code uses length - 1; add an
early check for length == 0 after computing unsigned length and handle it by
calling ERR_INVALID_ARG_TYPE with a safe empty/placeholder expected-type (e.g.,
an empty string or "[]") so no index math or toString() is performed; modify the
branch that currently handles length == 1 / length - 1 to first return when
length == 0 using the same ERR_INVALID_ARG_TYPE path so expected_types and any
subsequent indexing are never accessed when empty.

In `@src/js/internal/http2/util.ts`:
- Around line 559-562: The function assertWithinRange allows NaN because typeof
NaN === "number" and comparisons with NaN are false; update assertWithinRange to
explicitly reject NaN by adding a check (e.g., Number.isNaN(value)) to the
existing conditional so that any NaN value triggers the throw of
$ERR_HTTP2_INVALID_SETTING_VALUE_RangeError with the same message; locate the
function assertWithinRange and augment its if condition to include the NaN test.

In `@src/js/internal/test/binding.ts`:
- Around line 17-20: internalBinding currently returns an empty object when a
binding is missing which masks real missing/native failures; change the fallback
so that when bindings[name] is undefined the function calls and returns
process.binding(name) (preserving the existing behavior of omitting argument
validation), i.e., locate the internalBinding function and replace the final
"return {}" with "return process.binding(name)" so unknown bindings fall through
to Node's native process.binding.

In `@test/js/node/child_process/child_process-node.test.js`:
- Around line 674-678: The test currently asserts the full error.message string
using arg?.toString(), but Node formats the received value with its own
determineSpecificType rendering so the exact text can differ; update the
assertion for fork(...) to instead check error.code === "ERR_INVALID_ARG_TYPE"
and error.name === "TypeError" and assert the message matches a looser pattern
(e.g. use a RegExp like /^The "args" argument must be an instance of Array\.
Received .+/) rather than comparing to arg?.toString() so the test no longer
depends on determineSpecificType formatting.

In `@test/js/node/test/parallel/test-http2-misc-util.js`:
- Around line 19-21: The test's forEach loop is calling sessionName(2) instead
of using the loop variable i, so only the value 2 is actually tested; update the
assertion to call sessionName(i) and keep the rest of the test unchanged (the
loop over [2, '', 'test', {}, [], true] should assert
strictEqual(sessionName(i), '<invalid>') to validate all invalid inputs);
reference: sessionName and the forEach callback variable i in
test-http2-misc-util.js.

Additional findings (outside current diff — PR may have been updated during review):

• 🔴 src/jsc/bindings/ErrorCode.ts:102 — Inserting ERR_HTTP2_INVALID_CONNECTION_HEADERS mid-array (alphabetical position) violates the trailing comment in this file — discriminants are index-aligned with the checked-in src/jsc/ErrorCode.rs, which this PR doesn't touch. Every Rust ErrorCode constant ≥86 is now off-by-one against the regenerated C++ errors[] table and ErrorCode::COUNT (324) no longer matches; this is what's failing build-rust on all 8 platforms. Either append the entry after ERR_PROXY_TUNNEL at the end of the array, or regenerate src/jsc/ErrorCode.rs in this PR.

  Extended reasoning...

  What the bug is

  ERR_HTTP2_INVALID_CONNECTION_HEADERS is inserted at line 102 of src/jsc/bindings/ErrorCode.ts, between ERR_HTTP2_INFO_STATUS_NOT_ALLOWED and ERR_HTTP2_INVALID_HEADER_VALUE (the alphabetical position). However, the trailing comment in this same file (lines 334-335) explicitly states:

  // Appended (not alphabetical): discriminants are index-aligned with the checked-in Rust mirror (src/jsc/ErrorCode.rs) — only ever append here.

  The mid-array insertion shifts the index of every subsequent entry by +1, breaking the index alignment with src/jsc/ErrorCode.rs, which is not regenerated in this PR.

  The specific code path that triggers it

  There are two consumers of this array, with different update mechanisms:

  1. C++ side (ErrorCode+List.h / the errors[] table): regenerated at build time from ErrorCode.ts by src/codegen/generate-node-errors.ts. After this PR it will have 325 entries with ERR_HTTP2_INVALID_CONNECTION_HEADERS at index 86 and emit NODE_ERROR_COUNT = 325.
  2. Rust side (src/jsc/ErrorCode.rs): checked-in, not build-generated. Its header (lines 3-8) says "Discriminants MUST stay index-aligned with the C++ errors[] table so Bun__createErrorWithCode picks the correct ctor / name / code triple. Regenerate by re-running the inline extractor against ErrorCode.ts; do not hand-edit individual entries." At HEAD it still has:
     • HTTP2_INFO_STATUS_NOT_ALLOWED = ErrorCode(85) immediately followed by HTTP2_INVALID_HEADER_VALUE = ErrorCode(86) — no entry for the new code
     • pub const COUNT: u16 = 324
     • CODE_STR: [&str; ErrorCode::COUNT as usize] going straight from "ERR_HTTP2_INFO_STATUS_NOT_ALLOWED" to "ERR_HTTP2_INVALID_HEADER_VALUE"

  The PR's changed-files list does not include src/jsc/ErrorCode.rs, and git log -- src/jsc/ErrorCode.rs shows it was last touched before this PR.

  Why existing code doesn't prevent it

  The C++ generator runs every build, so it picks up the new entry automatically. The Rust mirror does not — it must be regenerated manually via the inline extractor mentioned in its header. The only guard is the in-source comment in ErrorCode.ts telling contributors to append, which was overlooked here. The robobun CI comment shows build-rust failing on all 8 platforms (linux x64/aarch64/musl/asan/baseline/android, macOS x64/aarch64) for commit 1969eee, consistent with a count/alignment static assertion firing during the Rust build.

  Step-by-step proof

  1. Before this PR, ErrorCode.ts index 85 = ERR_HTTP2_INFO_STATUS_NOT_ALLOWED, index 86 = ERR_HTTP2_INVALID_HEADER_VALUE. ErrorCode.rs mirrors this: ErrorCode(85) / ErrorCode(86). Both sides have 324 entries.
  2. This PR inserts ["ERR_HTTP2_INVALID_CONNECTION_HEADERS", TypeError] at TS index 86. Now TS index 86 = ERR_HTTP2_INVALID_CONNECTION_HEADERS, index 87 = ERR_HTTP2_INVALID_HEADER_VALUE, …, and the array has 325 entries.
  3. generate-node-errors.ts regenerates the C++ errors[] table to match: C++ index 86 → ERR_HTTP2_INVALID_CONNECTION_HEADERS, NODE_ERROR_COUNT = 325.
  4. ErrorCode.rs is unchanged: Rust still says HTTP2_INVALID_HEADER_VALUE = ErrorCode(86) and COUNT = 324.
  5. Any Rust caller passing ErrorCode::HTTP2_INVALID_HEADER_VALUE (value 86) into Bun__createErrorWithCode would now get an ERR_HTTP2_INVALID_CONNECTION_HEADERS instance — and likewise every one of the ~238 codes from index 86 onward is off by one. The COUNT mismatch (324 vs 325) trips the build-time check, which is why build-rust is red across the board.

  Impact

  The PR cannot build on any platform (all 8 build-rust jobs fail). If the count check were somehow bypassed, every Rust-originated error from index 86 onward would surface with the wrong .code / constructor / name at runtime.

  Fix

  Either of:

  • Append instead of insert: move the new entry to the end of the array, after ["ERR_PROXY_TUNNEL", Error] (in the section already marked "Appended (not alphabetical)"). This requires no Rust change.
  • Regenerate the Rust mirror: re-run the inline extractor against ErrorCode.ts and include the updated src/jsc/ErrorCode.rs (with the new entry at 86, all subsequent discriminants +1, COUNT = 325, and the new CODE_STR row) in this PR.

  The first option is the minimal change and keeps the file's stated invariant.

No issues found and CI is green; deferring to a human since the ERR_INVALID_ARG_TYPE wording change in ErrorCode.cpp is a cross-cutting behaviour change worth a second pair of eyes.